Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove extra rollbacks from Avian #562

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cBournhonesque
Copy link
Owner

With the move from XPBD to Avian, we noticed that even with a single player we have rollbacks, especially on contacts/collisions.
This should not happen normally because the client/server have access to the same inputs and the simulations are deterministic.

It seems to be because Avian maintains some stateful data:

  • sleeping: the entity is sleeping and is ignored by the physics engine
  • warm start: the collision solver use the previous frame's collisions

We have 2 solutions:

  • remove sleeping and disable warm start
  • add rollback for xpbd stateful resources, like Collisions and Sleeping

@ambiso
Copy link
Contributor

ambiso commented Oct 1, 2024

For the avian_3d_character example the same code (disabling sleeps and warm start) seems to completely mess up the simulation (at least with the 1 second replication interval that I'm testing with).

I tried additionally syncing the Collisions resource like this, but to no avail:

        // in protocol.rs
        app.register_resource::<CollisionInternals>(ChannelDirection::ServerToClient);
        app.add_systems(FixedPreUpdate, write_collisions_to_actual_collisions);
        app.add_systems(FixedPostUpdate, store_collisions);
        app.insert_resource(CollisionInternals(IndexMap::with_hasher(fxhash::FxBuildHasher::default()))); // needs `cargo add fxhash`
// [...]

#[derive(Serialize, Deserialize, Debug, Resource)] // needs `cargo add indexmap --features serde`
struct CollisionInternals(IndexMap<(Entity, Entity), Contacts, fxhash::FxBuildHasher>);

fn write_collisions_to_actual_collisions(
    collision_internals: Res<CollisionInternals>,
    mut collisions: ResMut<Collisions>,
) {
    *collisions.get_internal_mut() = collision_internals.0.clone();
}

fn store_collisions(
    collision_internals: ResMut<CollisionInternals>,
    collisions: Res<Collisions>,
) {
    collision_internals.into_inner().0 = collisions.get_internal().clone();
}

the cubes are still teleporting a bit every replication interval

@cBournhonesque
Copy link
Owner Author

For the avian_3d_character example the same code (disabling sleeps and warm start) seems to completely mess up the simulation (at least with the 1second replication interval that I'm testing with).

I tried syncing additional the Collisions resource like this, but to no avail:

        // in protocol.rs
        app.register_resource::<CollisionInternals>(ChannelDirection::ServerToClient);
        app.add_systems(FixedPreUpdate, write_collisions_to_actual_collisions);
        app.add_systems(FixedPostUpdate, store_collisions);
        app.insert_resource(CollisionInternals(IndexMap::with_hasher(fxhash::FxBuildHasher::default()))); // needs `cargo add fxhash`
// [...]

#[derive(Serialize, Deserialize, Debug, Resource)] // needs `cargo add indexmap --features serde`
struct CollisionInternals(IndexMap<(Entity, Entity), Contacts, fxhash::FxBuildHasher>);

fn write_collisions_to_actual_collisions(
    collision_internals: Res<CollisionInternals>,
    mut collisions: ResMut<Collisions>,
) {
    *collisions.get_internal_mut() = collision_internals.0.clone();
}

fn store_collisions(
    collision_internals: ResMut<CollisionInternals>,
    collisions: Res<Collisions>,
) {
    collision_internals.into_inner().0 = collisions.get_internal().clone();
}

the cubes are still teleporting a bit every replication interval

Thanks for trying this!
The avian 2d seems to have fewer rollbacks, so it might be something else; maybe a problem with the 3d example itself?

@ambiso
Copy link
Contributor

ambiso commented Oct 1, 2024

Probably this is missing an implementation of MapEntities, however even with a app.add_map_entities::<CollisionInternals>(); that implementation is not called. It appears that there's a ComponentRegistry and a MessageRegistry but no ResourceRegistry that takes care of resources?

Updated code:
        app.register_resource::<CollisionInternals>(ChannelDirection::ServerToClient);
        app.add_map_entities::<CollisionInternals>();
        app.add_systems(FixedPreUpdate, write_collisions_to_actual_collisions);
        app.add_systems(FixedPostUpdate, store_collisions);
        app.insert_resource(CollisionInternals(None));
// [...]
#[derive(Serialize, Deserialize, Debug, Resource, Clone)]
struct CollisionInternals(Option<IndexMap<(Entity, Entity), Contacts, fxhash::FxBuildHasher>>);

impl MapEntities for CollisionInternals {
    fn map_entities<M: EntityMapper>(&mut self, entity_mapper: &mut M) {
        panic!("Nice! MapEntities was called!");
        let mut new_map = IndexMap::with_capacity_and_hasher(
            self.0.as_ref().map_or(0, |x| x.len()),
            fxhash::FxBuildHasher::default(),
        );
        if let Some(inner) = self.0.take() {
            for ((e1, e2), value) in inner.into_iter() {
                new_map.insert(
                    (entity_mapper.map_entity(e1), entity_mapper.map_entity(e2)),
                    value,
                );
            }
            self.0 = Some(new_map);
        }
    }
}

fn write_collisions_to_actual_collisions(
    collision_internals: Res<CollisionInternals>,
    mut collisions: ResMut<Collisions>,
) {
    if let Some(collision_internals) = &collision_internals.0 {
        *collisions.get_internal_mut() = collision_internals.clone();
    }
}

fn store_collisions(collision_internals: ResMut<CollisionInternals>, collisions: Res<Collisions>) {
    collision_internals.into_inner().0 = Some(collisions.get_internal().clone());
}

@cBournhonesque
Copy link
Owner Author

cBournhonesque commented Oct 2, 2024

Resources are replicated as messages, so they are part of the MessageRegistry: https://github.com/cBournhonesque/lightyear/blob/main/lightyear/src/protocol/message.rs#L312

But rolling back resources is not supported currently

EDIT: actually it is in main: #622

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants